Merge changes from tls-channel to prevent accidentally calling SSLEng… #1726
Conversation
…ine.beginHandshake more than once. JAVA-5797
There was a problem hiding this comment.
Pull Request Overview
This PR merges upstream changes to prevent unintended duplicate calls to SSLEngine.beginHandshake by replacing the negotiated flag with separate handshakeStarted and handshakeCompleted flags.
- Replaces the "negotiated" flag with "handshakeStarted" and "handshakeCompleted".
- Adds a guard to prevent multiple invocations of SSLEngine.beginHandshake while retaining backward compatibility.
driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR merges upstream changes from the tls-channel project to prevent unintended SSLEngine renegotiation by introducing explicit handshake state tracking.
- Replace the single
negotiatedflag withhandshakeStartedandhandshakeCompletedfor clearer handshake lifecycle management. - Add a guard in
doHandshake()to skipengine.beginHandshake()on subsequent calls when not forcing a renegotiation. - Ensure
initSessionCallbackis invoked once per completed handshake.
Comments suppressed due to low confidence (4)
driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:162
- Consider adding a Javadoc comment explaining the role of the
handshakeStartedflag to clarify its lifecycle and prevent future confusion.
private boolean handshakeStarted = false;
driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:164
- Consider documenting the
handshakeCompletedflag, indicating that it tracks whether the TLS handshake has finished to improve code readability.
private volatile boolean handshakeCompleted = false;
driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:531
- Consider adding a unit test for the
force=truehandshake path afterhandshakeCompletedto verify that the handshake restarts correctly and doesn’t skip necessary steps.
if (!force && handshakeCompleted) {
driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:162
- [nitpick] You might consolidate
handshakeStartedandhandshakeCompletedinto a single enum state (e.g., NOT_STARTED, IN_PROGRESS, COMPLETED) to reduce complexity and prevent inconsistent flag usage.
private boolean handshakeStarted = false;
|
|
||
| @Test | ||
| @DisplayName("should not call beginHandshake more than once during TLS session establishment") | ||
| void shouldNotCallBeginHandshakeMoreThenOnceDuringTlsSessionEstablishment() throws Exception { |
There was a problem hiding this comment.
Added a test case, as upstream didn't include one to cover this change.
…ine (mongodb#1726) - Perform handshake after marking handshake started. - Add an integration test case, as upstream didn't include one to cover this change. JAVA-5797
See related bug report: marianobarrios/tls-channel#197
The PR from upstream has been manually merged: marianobarrios/tls-channel#201
JAVA-5797